Skip to content

Use ASCII JSON responses for proxy compatibility#1645

Open
leovefy wants to merge 2 commits into
runhey:devfrom
leovefy:codex/cobra-07-ascii-json-response
Open

Use ASCII JSON responses for proxy compatibility#1645
leovefy wants to merge 2 commits into
runhey:devfrom
leovefy:codex/cobra-07-ascii-json-response

Conversation

@leovefy

@leovefy leovefy commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Split from leovefy/cobra.

This PR contains a single commit: 28a71a9.
Base: runhey/dev.

Summary by Sourcery

将 FastAPI 应用配置为在所有响应和内部错误处理中使用仅包含 ASCII 字符的 JSON 响应类,以提升与代理的兼容性。

New Features:

  • 引入自定义的 ASCIIJSONResponse 类,以仅使用 ASCII 字符序列化 JSON,从而兼容特定代理。

Enhancements:

  • 将 FastAPI 应用的默认响应类和全局异常处理器设置为使用新的 ASCIIJSONResponse,以确保输出的 JSON 一致。
Original summary in English

Summary by Sourcery

Configure the FastAPI application to use an ASCII-only JSON response class for all responses and internal error handling to improve proxy compatibility.

New Features:

  • Introduce a custom ASCIIJSONResponse class that serializes JSON with ASCII-only characters for compatibility with certain proxies.

Enhancements:

  • Set the FastAPI app's default response class and global exception handler to use the new ASCIIJSONResponse for consistent JSON output.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - 我发现了 1 个问题

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="module/server/app.py" line_range="24-33" />
<code_context>
 from starlette.staticfiles import StaticFiles


+class ASCIIJSONResponse(JSONResponse):
+    """Serialize non-ASCII characters as ``\\uXXXX`` for proxy compatibility."""
+
+    def render(self, content) -> bytes:
+        return json.dumps(
+            content,
+            ensure_ascii=True,
+            allow_nan=False,
+            indent=None,
+            separators=(",", ":"),
+        ).encode("utf-8")
+
+
</code_context>
<issue_to_address>
**suggestion:** 建议复用 JSONResponse 的编码管线,而不是在这里直接硬编码调用 json.dumps。

使用直接调用 `json.dumps` 的方式重写 `render` 会绕过 JSONResponse 的编码管线(例如 `jsonable_encoder`、应用级自定义 `dumps`、未来切换到 `orjson` 等)。为了让这个响应与框架的其他部分保持一致,建议考虑委托给 `super().render`,只调整必要的参数(比如 `ensure_ascii`),或者重写 JSONResponse 在执行 dumps 时所使用的相关钩子,而不是重新实现完整的 render 逻辑。

建议实现:

```python
from pathlib import Path
import json

from contextlib import asynccontextmanager

from starlette.staticfiles import StaticFiles
from fastapi.responses import JSONResponse
from fastapi.encoders import jsonable_encoder


class ASCIIJSONResponse(JSONResponse):
    """Serialize non-ASCII characters as ``\\uXXXX`` for proxy compatibility."""

    def render(self, content) -> bytes:
        # Reuse FastAPI's JSON encoding pipeline (jsonable_encoder, etc.)
        encoded_content = jsonable_encoder(content)
        return json.dumps(
            encoded_content,
            ensure_ascii=True,
            allow_nan=False,
            indent=None,
            separators=(",", ":"),
        ).encode("utf-8")

```

1. 如果你的项目已经在本文件其他位置导入了 `JSONResponse` 和/或 `jsonable_encoder`,请调整导入部分以避免重复导入,并保持一致的导入风格。
2. 如果你使用了自定义的 JSON dumps 函数(例如通过应用级配置或 `ORJSONResponse`),并且希望 `ASCIIJSONResponse` 也参与该配置,可以考虑:
   -`__init__` 中接受一个 `json_dumps` 可调用对象(默认使用应用配置的 dumps);
   -`render` 中使用该可调用对象替代标准的 `json.dumps`,并在可调用对象支持的前提下,通过关键字参数传入 `ensure_ascii=True`。
</issue_to_address>

Sourcery 对开源项目免费——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="module/server/app.py" line_range="24-33" />
<code_context>
 from starlette.staticfiles import StaticFiles


+class ASCIIJSONResponse(JSONResponse):
+    """Serialize non-ASCII characters as ``\\uXXXX`` for proxy compatibility."""
+
+    def render(self, content) -> bytes:
+        return json.dumps(
+            content,
+            ensure_ascii=True,
+            allow_nan=False,
+            indent=None,
+            separators=(",", ":"),
+        ).encode("utf-8")
+
+
</code_context>
<issue_to_address>
**suggestion:** Consider reusing JSONResponse’s encoding pipeline rather than hard-coding json.dumps here.

Overriding `render` with a direct `json.dumps` call bypasses JSONResponse’s encoding pipeline (e.g., `jsonable_encoder`, app-level custom `dumps`, future switches to `orjson`). To keep this response consistent with the rest of the framework, consider delegating to `super().render` and only adjusting the necessary parameters (such as `ensure_ascii`), or overriding the relevant hook used by JSONResponse for dumps instead of reimplementing the full render logic.

Suggested implementation:

```python
from pathlib import Path
import json

from contextlib import asynccontextmanager

from starlette.staticfiles import StaticFiles
from fastapi.responses import JSONResponse
from fastapi.encoders import jsonable_encoder


class ASCIIJSONResponse(JSONResponse):
    """Serialize non-ASCII characters as ``\\uXXXX`` for proxy compatibility."""

    def render(self, content) -> bytes:
        # Reuse FastAPI's JSON encoding pipeline (jsonable_encoder, etc.)
        encoded_content = jsonable_encoder(content)
        return json.dumps(
            encoded_content,
            ensure_ascii=True,
            allow_nan=False,
            indent=None,
            separators=(",", ":"),
        ).encode("utf-8")

```

1. If your project already imports `JSONResponse` and/or `jsonable_encoder` elsewhere in this file, adjust the import section to avoid duplicates and keep a consistent import style.
2. If you use a custom JSON dumps function (e.g. via app-level configuration or `ORJSONResponse`), and you want `ASCIIJSONResponse` to participate in that configuration, consider:
   - Accepting a `json_dumps` callable in `__init__` (defaulting to the app's configured dumps), and
   - Using that callable instead of the standard `json.dumps` inside `render`, while still passing `ensure_ascii=True` via keyword arguments if the callable supports it.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread module/server/app.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant